[ECO-5085] Validate spec coverage tags#243
Conversation
These tests became incorrect in 780be56, in which: - the implementation of the spec points that these tests claim to test was removed - these tests were changed to not actually test anything - the total on the @specOneOf points was not updated to reflect the new number of tests
Part of #96.
Check there are no contradictory tags. Resolves #96.
WalkthroughThe changes refactor spec coverage validation logic by adding three new error cases to report issues related to Changes
Sequence Diagram(s)sequenceDiagram
participant SP as SpecPoint
participant GC as generateCoverage
participant EH as Error Handling
SP->>GC: Invoke generateCoverage(specPoint, conformanceTags)
GC->>EH: Check for multiple conformance tag types
EH-->>GC: Report error if multiple types exist
GC->>EH: Validate specOneOf tag totals
EH-->>GC: Report error if totals mismatch
GC->>EH: Validate specOneOf tag indices
EH-->>GC: Report error if indices are incorrect
GC-->>SP: Return SpecPointCoverage object upon successful validation
Assessment against linked issues
Possibly related PRs
Suggested reviewers
Poem
Tip ⚡🧪 Multi-step agentic review comment chat (experimental)
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
Sources/BuildTool/BuildTool.swift (1)
479-550: Optional enhancement to gather multiple errors per spec point.
The logic to compute coverage and throw on the first encountered issue is valid, but collecting all errors for a single spec point would provide more comprehensive feedback. If necessary, you could store all violations and throw a combined error for an improved user experience.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
Sources/BuildTool/BuildTool.swift(5 hunks)Tests/AblyChatTests/MessageTests.swift(0 hunks)
💤 Files with no reviewable changes (1)
- Tests/AblyChatTests/MessageTests.swift
⏰ Context from checks skipped due to timeout of 90000ms (12)
- GitHub Check: Example app, tvOS (Xcode 16)
- GitHub Check: Example app, iOS (Xcode 16)
- GitHub Check: Generate code coverage (Xcode 16)
- GitHub Check: Example app, macOS (Xcode 16)
- GitHub Check: Xcode,
releaseconfiguration, tvOS (Xcode 16) - GitHub Check: Xcode, tvOS (Xcode 16)
- GitHub Check: Xcode,
releaseconfiguration, iOS (Xcode 16) - GitHub Check: SPM (Xcode 16)
- GitHub Check: SPM,
releaseconfiguration (Xcode 16) - GitHub Check: Xcode, iOS (Xcode 16)
- GitHub Check: Xcode,
releaseconfiguration, macOS (Xcode 16) - GitHub Check: Xcode, macOS (Xcode 16)
🔇 Additional comments (3)
Sources/BuildTool/BuildTool.swift (3)
282-284: The new error cases are well-structured and enhance clarity.
These typed errors help communicate precise validation failures for spec coverage issues.
450-450: Good addition of descriptive comment.
This clarifies the logic flow and improves the overall readability for future maintainers.
462-466: Nice separation of concerns for coverage generation.
Mapping each testable spec point to a coverage result improves readability and maintainability.
2 participants
This catches the following:
@specOneOf(m/n)tags where the total number of such tags for a spec point doesn't match the stated total, e.g.@specOneOf(m/n)tags where themvalues don't give the range1...n, e.g.Multiple tag types for a single spec point, e.g.
As part of this, I've removed a test file that contained incorrect tests (and incorrect tags).
Resolves #96.
Summary by CodeRabbit